use a tuple for function arguments #4321
Conversation
This is follow-up from PR #4317. This introduces tuple_exprt. function_application_exprt now uses tuple_exprt for the argument operand, instead of an expression with empty ID.
kroening
requested review from
peterschrammel,
pkesseli,
smowton and
tautschnig
as code owners
| @@ -186,6 +186,15 @@ inline void validate_expr(const factorial_power_exprt &value) | |||
| validate_operands(value, 2, "Factorial power must have two operands"); | |||
| } | |||
|
|
|||
| class tuple_exprt : public multi_ary_exprt | |||
There was a problem hiding this comment.
Would it be worth moving this to std_expr.h and use it in those places where we currently do use ID_arguments?
There was a problem hiding this comment.
Alternatively I would suggest putting all the .*_exprt classes in a new module (separate from util) and have one class per header file, but that's a much bigger change.
There was a problem hiding this comment.
I like this idea, but this is a very intrusive change -- a lot of code in util already depends on std_expr.h:
$ git grep 'include.*std_expr.h' src/util/ | wc -l
43
There was a problem hiding this comment.
I think that the arguments/parameters of a C/C++ function are a very different thing from the arguments/parameters of a mathematical function. In particular, there is no tuple building going on in C/C++ or Java.
There was a problem hiding this comment.
@romainbrenguier getting slightly off-topic, we could separate the 'irep/expression' related things from the 'non irep utilities', say making a directory named "irep". There could be dummy headers for a transition period.
There was a problem hiding this comment.
This PR failed Diffblue compatibility checks (cbmc commit: a8e67cb).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/103063401
Status will be re-evaluated on next push.
Common spurious failures:
-
the cbmc commit has disappeared in the mean time (e.g. in a force-push)
-
the author is not in the list of contributors (e.g. first-time contributors).
-
the compatibility was already broken by an earlier merge.
This is follow-up from PR #4317.
This introduces
tuple_exprt.function_application_exprtnow usestuple_exprtfor the argument operand,instead of an expression with empty ID.